useStoreListener hook by everdimension · Pull Request #2 · nanostores/react

everdimension · 2021-10-30T09:54:06Z

Motivation:
A common pattern is to react to some store change in an effect:

function MyComponent() {
  const { field } = useStore(myStore);

  useEffect(() => {
    if (field === 'satisfies-some-condition') {
      triggerAnotherAction();
    }
  }, [field]);

  return (
    ...other components...
  )
}

While this does the job, unfortunately it makes the whole component re-render when store changes, even if the field doesn't yet satisfy our condition.

Solution here would be to create a hook that is designed to only trigger effects and not interfere with the render phase:

function MyComponent() {
  useStoreListener(myStore, {
    keys: 'field',
    listener: (value, changed) => {
      if (changed === 'satisfies-some-condition') {
        triggerAnotherAction()
      }
    }
  })

  return (
    ...other components...
  )
}

That way, we can use the familiar effect pattern, but avoid unwanted re-renders and calls of other hooks in the component

If you decide this is a good feature, I'll finish the pull request by updating types, tests and docs, thanks!

ai · 2021-10-30T10:04:22Z

The origin idea of Nano Stores is to move logic from React components.

In general, I see the nanostores’s way to solve this task in:

import { listenKeys } from 'nanostores'

function bindAnotherAction () {
  const unbind = listenKeys(myStore, ['field'], value => {
    if (value.field === 'satisfies-some-condition') {
      triggerAnotherAction()
    }
  })
  return unbind
}

function MyComponent() {
  useEffect(bindAnotherAction)
}

Do you have reasons to do have this logic in React component?

everdimension · 2021-10-31T08:37:37Z

I think this is more of a idealistic vs pragmatic approach

I.e. in a perfect world it might be a good practice to make sure that no local component logic interferes with logic that is handled by the stores (outside react)

In practice, we just wanna be able to do stuff and have the tools that help us.

Extracting bindAnotherAction has a problem of not only being tedious, but mainly the issue is that the listener might close over other variables and state in the component. You lose access to it once you extract it outside the component.

ai · 2021-10-31T10:26:47Z

OK, let’s add tests and docs.

I agree that sometimes people may want old approach.

everdimension · 2021-11-07T19:30:57Z

Hi again!
Added some tests and types.

I'm not sure whether the keys option works only for the map store, or for the atom store as well if its value is an object.

If everything's fine the next thing to do is to update the docs

everdimension · 2021-11-07T19:34:10Z

index.d.ts


+type Listener<SomeStore extends Store> = (
+  value: StoreValue<SomeStore>,
+  changed?: SomeStore extends MapStore ? StoreValue<SomeStore> : never


Does this parameter only have a value if the store is map store?

And what is its value if the store value is a primitive?

Does this parameter only have a value if the store is map store?

Yeap

And what is its value if the store value is a primitive?

In this case you do not have store.setKey and we have no easy way to detect changes

Well I was asking because of this: https://github.com/nanostores/react/pull/2/files#r744618456

everdimension · 2021-11-08T11:04:46Z

index.js

+  React.useEffect(() => {
+    let listener = (value, changed) => listenerRef.current(value, changed)
+    if (opts.leading) {
+      listener(store.get(), null)


Is it correct to pass null here?

I think it is better to call listener(store.get())

Ok, thanks!

ai · 2021-11-08T21:07:51Z

LGTM. Do you want to add anything else?

everdimension · 2021-11-08T21:51:33Z

Great, thanks!
We should update the docs, too. I can do it as a separate pull-request or as part of this one.

ai · 2021-11-08T22:41:44Z

Let’s update docs in this PR

dartess · 2023-05-07T16:41:04Z

index.js

  return store.get()
 }
+
+export function useStoreListener(store, opts = {}) {


it is worth moving the new hook into a separate file: this way it will not affect those who do not need it.

dartess · 2023-05-07T16:41:49Z

index.js

+    }
+  }, [store, '' + opts.keys, opts.leading])
+
+  return null


the return does nothing, it seems it can be removed.

everdimension force-pushed the feature/use-store-listener branch from 6b7b6ae to dea7127 Compare October 30, 2021 09:54

useStoreListener hook

529e439

everdimension force-pushed the feature/use-store-listener branch from dea7127 to 16ffa43 Compare November 7, 2021 19:28

everdimension commented Nov 7, 2021

View reviewed changes

everdimension commented Nov 8, 2021

View reviewed changes

useStoreListener types and tests

a37b553

everdimension force-pushed the feature/use-store-listener branch from 16ffa43 to a37b553 Compare November 8, 2021 11:23

ai approved these changes Nov 8, 2021

View reviewed changes

dartess reviewed May 7, 2023

View reviewed changes

Uh oh!

Conversation

everdimension commented Oct 30, 2021

Uh oh!

ai commented Oct 30, 2021

Uh oh!

everdimension commented Oct 31, 2021

Uh oh!

ai commented Oct 31, 2021

Uh oh!

everdimension commented Nov 7, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ai commented Nov 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

everdimension commented Nov 8, 2021

Uh oh!

ai commented Nov 8, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ai commented Nov 8, 2021 •

edited

Loading